Add symbol and type highlighting to F# diagnostics - #20097
Conversation
This comment has been minimized.
This comment has been minimized.
81e1ec1 to
77b7272
Compare
|
🔍 Tooling Safety Check — Affects-Bootstrap, Affects-Build-Infra, Affects-Compiler-Output
|
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ef5fe96 to
4bb1086
Compare
|
This is ready. |
| <Link>FSStrings.resx</Link> | ||
| <LogicalName>FSStrings.resources</LogicalName> | ||
| </EmbeddedResource> | ||
| <Compile Include="Utilities\NullHelpers.fs" CompileOrder="CompileFirst" /> |
There was a problem hiding this comment.
Why is the CompileOrder necessary?
| FSharp.Compiler.Text.RichTextModule: FSharp.Compiler.Text.RichText mkAlias(System.String) | ||
| FSharp.Compiler.Text.RichTextModule: FSharp.Compiler.Text.RichText mkClass(System.String) | ||
| FSharp.Compiler.Text.RichTextModule: FSharp.Compiler.Text.RichText mkDelegate(System.String) | ||
| FSharp.Compiler.Text.RichTextModule: FSharp.Compiler.Text.RichText mkEnum(System.String) |
There was a problem hiding this comment.
Are all the maker functions public by intention? Who is the target user from different assemblies for making diagnostics?
| @@ -11140,6 +11198,7 @@ FSharp.Compiler.Text.TextTag+Tags: Int32 TypeParameter | |||
| FSharp.Compiler.Text.TextTag+Tags: Int32 Union | |||
| FSharp.Compiler.Text.TextTag+Tags: Int32 UnionCase | |||
| FSharp.Compiler.Text.TextTag+Tags: Int32 UnknownEntity | |||
There was a problem hiding this comment.
What is the distinction between UnknownEntity and UnresolvedName ?
| [<Fact>] | ||
| let ``Can get help for FSComp.SR.considerUpcast`` () = | ||
| match FSharp.Compiler.Interactive.FsiHelp.Logic.Quoted.tryGetHelp <@ FSComp.SR.considerUpcast @> with | ||
| match FSharp.Compiler.Interactive.FsiHelp.Logic.Quoted.tryGetHelp <@ (FSComp.SR.considerUpcast: string * string -> int * string) @> with |
There was a problem hiding this comment.
Why do we have to type annotate now?
Is this change source-breaking for some code relying on type inference?
| <PropertyGroup Condition="'$(MSBuildProjectName)' == 'FSharp.Compiler.Service' AND ('$(Configuration)' == 'Proto' OR !Exists('$(ProtoOutputPath)'))"> | ||
| <LocalFSharpBuildAssemblyFile>$(ArtifactsDir)bin\FSharp.Build\$(Configuration)\netstandard2.0\FSharp.Build.dll</LocalFSharpBuildAssemblyFile> | ||
| </PropertyGroup> | ||
| <UsingTask TaskName="FSharpEmbedResourceText" AssemblyFile="$(LocalFSharpBuildAssemblyFile)" Override="true" TaskFactory="TaskHostFactory" Condition="'$(LocalFSharpBuildAssemblyFile)' != ''" /> |
There was a problem hiding this comment.
Please consolidate with the UsingTask and Override above this added block.
We only need one such block.
To better stand the test of time, the MSBuildProjectName condition could instead be moved to a dedicated property ("UseBoostrappedFsharpBuildTasks") declared at project level.
Meaning the block in this file would keep on living, but the property at individual .fsproj project(s) could be added/removed as the needs evolve and also as SDK tasks become sufficient.
| enabledByLangFeature: bool | ||
|
|
||
| /// Creates a diagnostic exception whose text comes via SR.* | ||
| val Error: (int * string) * range -> exn |
There was a problem hiding this comment.
Would make sense marking this one Obsolete?
Or maybe a different name indicating a "tagless" string?
Some heuristic - if a diagnostic message has format holes, it should almost certainly be routed trough a RichError, not via classical Error? (or did I get it wrong?)
| | [||] -> "" | ||
| | [| part |] -> part.Text | ||
| | parts -> | ||
| let buf = StringBuilder() |
There was a problem hiding this comment.
Can we allocate with a calculated/estimated upper bound size?
Implements highlighting for symbols and types in diagnostic messages to be used by IDEs and tools like F# Interactive.
To make it possible, this PR introduces
RichTexttype which wrapsTaggedText[]uses and making the code simpler to work with in existing FCS APIs like tooltips.The FSComp helpers generation is changed in a way that the same format works without changes, and only an additional
RichTextflag is needed in the fsproj.